Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(gatsby): add Offer a base Page props type which folks can extend #21542

Merged
merged 3 commits into from
Feb 29, 2020
Merged

chore(gatsby): add Offer a base Page props type which folks can extend #21542

merged 3 commits into from
Feb 29, 2020

Conversation

orta
Copy link
Contributor

@orta orta commented Feb 18, 2020 •
edited
Loading

Description

Once I started using shared fragments, all the auto-generate types tooling broke, so I went manual.

I started using this type in TypeScript v2 and figured it'd be better that I give it to Gatsby instead. Effectively gives people a base type to build on top of for describing the data in their

This is how I define my types which have both a query and custom context from createPage.

Screen Shot 2020-02-17 at 10 46 15 PM

( and yah yah, lang vs langKey - I couldn't find docs on langKey so I just used my own everywhere)

Documentation

When there's official TS docs, this should go in as a way to describe your own props - I included a bunch of examples inline which you can copy & paste later

Discussion

I do not know what these are/do:

  • "*" could be on the props, but I included it because it was on the older one (it doesn't show up in any of my props at runtime)

  • pageResources - I don't get what this does, and couldn't figure it out from some googling and reading docs - so I didn't document it and left it ambiguous but there

@orta orta requested a review from a team as a code owner February 18, 2020 04:02
@wardpeet
Copy link
Contributor

"*" could be on the props, but I included it because it was on the older one (it doesn't show up in any of my props at runtime)

We should be able to safely remove this.

pageResources - I don't get what this does, and couldn't figure it out from some googling and reading docs - so I didn't document it and left it ambiguous but there

This is an object that holds info about what the page dependencies are, component, data, ... This should have a fixed type

interface PageResources {
  component: React.Component
    json: {
      data: DataType
      pageContext: PageContextType
    },
    page: {
      componentChunkName: string,
      path: string,
      webpackCompilationHash: string,
      matchPath?: string,
    },
}

@wardpeet wardpeet added the status: awaiting author response Additional information has been requested from the author label Feb 18, 2020
@orta
Copy link
Contributor Author

orta commented Feb 18, 2020

Updated - thanks!

@orta @wardpeet
Update packages/gatsby/index.d.ts
f8224f3 
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
wardpeet
wardpeet approved these changes Feb 29, 2020
View reviewed changes
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @orta!

@wardpeet wardpeet merged commit abd228e into master Feb 29, 2020
@wardpeet wardpeet deleted the dts_page_props_ branch February 29, 2020 18:17
@wardpeet wardpeet changed the title Offer a base Page props type which folks can extend chore(gatsby): add Offer a base Page props type which folks can extend Feb 29, 2020
@wardpeet
Copy link
Contributor

wardpeet commented Mar 2, 2020

Fixed in gatsby@2.19.24

@adambrgmn adambrgmn mentioned this pull request Mar 9, 2020
Merged
pieh pushed a commit that referenced this pull request Mar 10, 2020
@adambrgmn
fix(gatsby): Update types for WrapPageElement*Args (#22120)
97fa23e 
* Update WrapPageElement*Args interface

PR #21542 introduced a new type PageProps which can also be used on WrapPageElement*Args interfaces.

* Add generics for data and context
This was referenced Mar 17, 2020
Open
Open
This was referenced Oct 6, 2020
Open
Open
@ajesse11x ajesse11x mentioned this pull request Feb 23, 2021
Open
@saurabharch saurabharch mentioned this pull request Feb 23, 2021
Open
@snyk-bot snyk-bot mentioned this pull request May 19, 2022
Open
@ajesse11x ajesse11x mentioned this pull request May 29, 2022
Open
This was referenced Jul 21, 2022
Open
Open
@ajesse11x ajesse11x mentioned this pull request Aug 24, 2022
Open
@saurabharch saurabharch mentioned this pull request Nov 27, 2023
Open
@ajesse11x ajesse11x mentioned this pull request Nov 29, 2023
Open
@ajesse11x ajesse11x mentioned this pull request Apr 16, 2024
Open
@saurabharch saurabharch mentioned this pull request Apr 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pieh pieh pieh left review comments

@wardpeet wardpeet wardpeet approved these changes

Assignees
No one assigned
Labels
status: awaiting author response Additional information has been requested from the author
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@orta @wardpeet @pieh